Skip to content

refactor: route _fs_namespace reads through getFsNamespace helper#250

Open
pablofullstory wants to merge 1 commit into
mainfrom
refactor/fs-namespace-helper
Open

refactor: route _fs_namespace reads through getFsNamespace helper#250
pablofullstory wants to merge 1 commit into
mainfrom
refactor/fs-namespace-helper

Conversation

@pablofullstory
Copy link
Copy Markdown

@pablofullstory pablofullstory commented May 28, 2026

Summary

DLO had four hard-coded window._fs_namespace reads, which silently break for
sites whose FullStory snippet uses a custom namespace (set via the
data-fs-namespace script attribute) without also setting the _fs_namespace
global. This PR centralizes the lookup behind a single helper that mirrors the
resolution order used by @fullstory/browser-api.

  • Add src/utils/fsNamespace.ts exporting getFsNamespace(win = window):
    data-fs-namespace on document.currentScript_fs_namespace global → 'FS'.
    Self-contained (no @fullstory/browser-api dependency); defensive try/catch
    around document access; empty-string attribute falls through to the global.
  • Replace the four direct reads with helper calls:
    • src/utils/logger.tsFullStoryAppender.log
    • src/operators/fsApi/fsApi.tsFSApiOperator.handleData
      (also tightens the throw message to Fullstory namespace is not a function)
    • src/target.tsDataLayerTarget constructor's "is this the FS object?" guard
    • src/embed/init.impl.tsattachDloFullStoryLifecycle

Behavior

  • Existing call sites that only had the _fs_namespace global still work — the
    helper falls back to the global, which is the same floor the old code had.
  • Sites that set data-fs-namespace="MyFS" on the FullStory snippet but do not
    set _fs_namespace now resolve correctly (this was the bug).
  • document.currentScript is only meaningful while a script tag is being
    evaluated synchronously. DLO calls the helper from deferred callbacks
    (appender log, operator handleData, lifecycle hook), so the attribute path
    is opportunistic; the global remains the practical fallback. This matches how
    @fullstory/browser-api's own getFsNamespace is shaped.

Test plan

  • npm test — 428 passing, including 8 new cases in test/fs-namespace.spec.ts
    covering: attribute wins over global, empty-string attribute is ignored,
    missing/null currentScript falls through, errors during document access
    are swallowed, default 'FS' floor, and undefined window.
  • npm run lint — clean (only pre-existing no-console warnings remain).
  • npm run build — clean.
  • Existing specs that seed window._fs_namespace = 'FS' (logger.spec.ts,
    lifecycle-embed.spec.ts, operator-fsApi.spec.ts) still pass via the
    helper's global fallback.

Note

Low Risk
Localized integration refactor with preserved global/FS fallbacks and new unit tests; no auth or data-path changes.

Overview
Introduces getFsNamespace so every FullStory client lookup uses the same resolution order as @fullstory/browser-api: data-fs-namespace on document.currentScript (when available), then _fs_namespace, then 'FS'. Document access is wrapped in try/catch; an empty script attribute is ignored.

Replaces four direct window._fs_namespace reads in FullStoryAppender, FSApiOperator, DataLayerTarget, and attachDloFullStoryLifecycle with the helper. Sites that only set the global behave as before; sites that set a custom namespace on the snippet but omit _fs_namespace should now resolve the correct client function.

Adds test/fs-namespace.spec.ts for precedence, deferred/null currentScript, errors, and the default floor.

Reviewed by Cursor Bugbot for commit 3bb3717. Bugbot is set up for automated code reviews on this repo. Configure here.

Comment thread src/utils/fsNamespace.ts
/**
* Resolves the FullStory client namespace on `window`.
*
* Mirrors the resolution order used by `@fullstory/browser-api`:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's say fs.js instead of @fullstory/browser-api since that's an internal package and this is theoretically an open source repo.

Copy link
Copy Markdown
Contributor

@ScottLNorvell ScottLNorvell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just the one comment about removing the reference to @fullstory/browser-api. (which I'm sure that silly LLM put there hahhaha)

Comment thread test/fs-namespace.spec.ts
});

it('prefers the script attribute over _fs_namespace when both are present', () => {
const win = makeWin({ scriptAttr: 'AttrWins', fsNamespace: 'GlobalLoses' });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duck_hunt_laugh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants